build: move the provisioning tail out of build and into provision - #251
build: move the provisioning tail out of build and into provision#251mobileoverlord wants to merge 5 commits into
Conversation
880b7c2 to
e15e385
Compare
mobileoverlord
changed the title
There was a problem hiding this comment.
🟡 Changes recommended
The moved provisioning tail currently has path/environment mismatches and provision still enforces a “provisionable build” gate with misleading messaging, which can cause provisioning failures or unnecessary rebuilds.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR relocates the “provisioning tail” (var image + stone OS bundle + Docker priming) out of runtime build and into runtime provision, introducing a dedicated renderer (commands/runtime/var_image.rs) and new stamp semantics to distinguish deployable build outputs from provisioning-only artifacts.
Changes:
- Add
commands/runtime/var_image.rsto render/run the provisioning tail fromprovision(with unit tests around rendering and bash validity). - Introduce a new
Provisionablestamp command/requirement and wire it into runtime build/provision flows. - Add a
--provisionableCLI flag plumbing and document the breaking change inCHANGELOG.md.
File summaries
| File | Description |
|---|---|
| src/utils/stamps.rs | Adds Provisionable stamp command plus constructors/requirements and tests. |
| src/main.rs | Adds --provisionable flags for build/runtime build and wires them into command builders. |
| src/commands/runtime/var_image.rs | New module rendering the provisioning tail script + tests. |
| src/commands/runtime/provision.rs | Runs the rendered provisioning tail before the provision hook; adds stamp gate helper/tests. |
| src/commands/runtime/mod.rs | Exposes the new var_image module. |
| src/commands/runtime/build.rs | Removes the provisioning tail from build output; optionally writes a provisionable stamp. |
| src/commands/build.rs | Plumbs provisionable through top-level build into runtime builds. |
| CHANGELOG.md | Documents the breaking change and migration guidance. |
Review details
Suppressed comments (2)
src/commands/runtime/provision.rs:225
- This gate refuses provisioning unless a
provisionablestamp exists, but this command now builds the var image and OS bundle itself later inexecute()(viavar_image::render), so the refusal can block an otherwise-validavocado provision. The error message is also misleading because it claims the artifacts are built byavocado build --provisionable, but the artifacts are produced here in provision.
// `avocado build` produces the deployable set only. The var image
// and the OS bundle — the artifacts stone flashes — come from the
// provisioning tail, which runs only under `--provisionable` and
// records its own stamp. Require that stamp, and require it current:
// a plain rebuild since then deleted those artifacts and the stamp
src/main.rs:1786
- Same issue as the top-level
buildcommand: this--provisionablehelp text says it builds provisioning artifacts, but the provisioning artifacts are now built duringruntime provision. The CLI docs should reflect what the flag actually does (or the flag should be removed if it’s no longer needed).
/// Also build the provisioning artifacts: the var image, the OS bundle
/// and Docker priming.
///
/// Without this, a build produces only what `avocado deploy` and
/// `avocado connect upload` consume. `avocado provision` requires a build
/// that used this flag.
- Files reviewed: 8/8 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
e15e385 to
9c8b901
Compare
|
@copilot review |
|
All four findings from the review are addressed as of
Two of those were bugs that would have shipped, and the gate one would have broken provisioning outright — thanks. New test Note for a re-review: the review summary above was re-timestamped by the force-push but its inline comments are outdated and its body is unchanged, so it reads as if nothing was addressed. |
There was a problem hiding this comment.
🟡 Changes recommended
Two newly added doc comments are misattached to the runs_on option, producing misleading user-facing help/docs for remote execution.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 2
- Review effort level: Lite
9c8b901 to
23355d6
Compare
|
Review round 2 addressed in
5 and 6 were caused by removing the Gate: full Two Copilot re-review requests have been accepted ( |
…mage in provision `avocado build` produced a var image and an OS bundle. The var image is only ever read by `provision`; the OS bundle is the OTA payload. The first cut of this change moved both to `provision`, which broke OTA on UKI platforms: the `avocado-build-<target>` hook *is* the kernel and initramfs there, so `build` emitted no boot artifacts, and the runtime manifest carried no `os_bundle`. Hardware-verified on QCS9075 — `deploy` reported success, updated the extensions, and left the board on its old kernel with /var still plaintext. Nothing reported that the OS had not been updated. The split is by consumer, not by phase: - Build tail: the stone include paths, device-tree overlays, the platform build hook, `stone bundle`, the `os_bundle` manifest patch and the re-sign after it. All of it produces or names OTA payload. It is spliced into the build script and inherits that script's variables and its `sign_amf` helper — which also fixes `sign_amf: command not found`, where the moved call resolved to nothing and provisioning died after the bundle was already built. - Provision: Docker priming and the var image. Verified that no provisioning script reads the bundle; `avocado-provision-<target>` and the UFS flow both inject raw images. Ordering matters as well as placement. The manifest id is minted by `manifest_section`, so a bundle produced afterwards by a separate command is patched into a document whose id has not changed — a device already holding that id skips the update, including the `os_bundle` that appeared inside it. Building the bundle in the same run that mints the id makes every build's manifest carry its own OS payload. `stone bundle` fails without a var partition size, because platform manifests declare `var` as `expand: "true"` with no size — that is the case `--partition-size` exists for. The var image does not exist at build time, so the size is declared from the staged tree with headroom, documented at the call site along with the condition that would make it a real number.
23355d6 to
1b2175d
Compare
`$AVOCADO_PREFIX/output/runtimes/<rt>` and `$AVOCADO_PREFIX/runtimes/<rt>` are two different trees. The first is where the SDK's stone tooling looks (`$AVOCADO_STONE_DATA_DIR`), where the device-tree overlay staging writes, and where `provision` keeps its state. `STONE_BUILD_DIR` and `STONE_AOS_OUTPUT` belong there and always did. A review read them as duplicating `OUTPUT_DIR` and "easy to drift", and I repointed them without checking that the two paths were the same tree. They are not. The build then wrote its stone outputs to the new location while `provision` kept unpacking the bootfiles tarball still sitting at the old one — so a flash wrote an OLD GPT and OLD firmware alongside a newly built system image, var image and ESP. On a build that had renamed the var partition label, the new initramfs looked for a label the freshly written GPT did not have and every boot ended in initrd emergency, with nothing pointing at the cause. `rm -rf "$STONE_BUILD_DIR"` was cleaning the wrong directory too, which is why the stale copy survived rebuild after rebuild. Found on hardware. Pinned by a test asserting both paths and asserting they are not under `$OUTPUT_DIR`, because the next reviewer will read them the same way.
Each branch in this stack appended its own `### Changed` / `### Fixed` block, so by the top of the stack Unreleased carried several of each. That breaks Keep-a-Changelog structure and renders as repeated sections. Merged per section, preserving entry order. No entry text changed.
The problem
This PR moves the var image to The PR says that stone gets the same inputs as before. On qemuarm64 stone What already worksTwo parts of the split need no change:
Why a derived manifest does not workOne option is to give the hook a manifest with The fix, part 1 — declare the partition at buildBefore the hook runs, the build script creates a sparse file at the path The file is sparse and uses the declared size that The The fix, part 2 — refresh what stone provisions fromPart 1 alone puts the placeholder on the device. It reads the runtime dir only to The board then gets a var partition of the correct size that holds only The build dir held a var image of 214548876 bytes that was 0 bytes on disk. So This does not go in the var section. That half must stay portable for LimitsThis is a stopgap. The hook does two different jobs: it stages OTA
The failure above is an argument for one of these. Two files with the same |
ChangesBranch src/commands/runtime/var_image.rs (+65)
src/commands/runtime/provision.rs (+79)
No other source file changes. No changes to meta-avocado. Not part of the fix
Test results
ValidationBuild-level A/B on qemuarm64, release 2024, channel edge, with the
On hardware:
|
avocado-build-<target> runs stone validate and stone create over the whole
manifest, and every platform manifest names a var image. Moving the var image
to provision left the hook with one input fewer, and it stops:
[ERROR] Validation failed. 1 file(s) not found:
device: rootdisk, image: var
A derived manifest with var removed does not work: the partition is
expand: "true" with no size, stone still needs one, and the hook passes no
--partition-size of its own, so the layout could differ between stone create
and stone provision.
Creates a sparse placeholder at the size stone bundle is already given. The
hook can resolve the partition; the costly work stays in provision. Guarded on
absence because a provision in the same project leaves a real image here.
This half is NOT sufficient alone -- see the following commit.
With the placeholder alone the board gets a var partition of the right size
holding zeros:
erofs: (device mmcblk0p7): cannot find valid erofs superblock
[FAILED] Failed to mount /var. You are in emergency mode.
Two files of this name exist under AVOCADO_PREFIX and stone resolves
first-match-wins across its -i dirs, so which reaches the device is not
visible from the code that flashes it. stone create filled the build dir
during build, when the only var image was the placeholder.
Replaces that copy with the real image, between the var section and the hook.
Hard link, copying if it crosses a mount. Not in the var section: that half
stays portable for provision --bundle, which carries no build tree.
Fails closed -- this shell has no set -e, and a silent skip ships a device
that drops to an emergency shell on first boot.
|
Pushed two commits fixing the var-image staging this PR introduced, both from @nicksinas's diagnosis above — thanks for the writeup, it named the failure and the fix precisely.
Part 1 is the sparse Part 2 replaces the stone build dir's copy with the real image, between the var section and the hook, hard link with a copy fallback. Kept out of the var section so One deviation from the writeup: the refresh fails closed. That shell runs without Two tests, both asserting the ordering, which is the whole point: Confirmed on hardware, on a different board than the reportAn RB3 Gen 2 hit the same root cause with a different symptom — The var image was at The warning about part 1 alone is worth repeating: a placeholder-only fix flashes successfully and leaves the board in emergency mode with a correctly-sized, zero-filled Note the |
Breaking change to what
avocado buildproduces. Read the first sectionbefore the diff.
What changes
avocado buildandavocado runtime buildnow produce the deployable set:the runtime manifest, the images and the elements. That is exactly what
avocado deployandavocado connect uploadread, and all either has everread.
The var image is a provisioning artifact — nothing but
avocado provisionconsumes it — so
provisionnow builds it itself, at its entry point, ahead ofthe
avocado-provision-<arch>hook. Stone is untouched: it is invokedexactly as before and handed exactly the inputs it has always been handed.
The OS bundle stays in
build, along with theavocado-build-<target>hookand the
os_bundlemanifest patch. See "Correction" below — an earlier revisionof this PR moved them too, and that broke OTA.
Migration
A pipeline that runs
avocado buildand then flashes will find no var image.Run
avocado provision, which produces it.There is deliberately no flag to restore the old behaviour. A flag on
buildwould be asking the user a question whose answer is always "whatever thenext command needs" — and the flag design that was prototyped here made that
concrete: it needed a stamp for
provisionto interrogate, and an else-branchthat deleted both the artifacts and that stamp, because otherwise a plain
rebuild after a provisionable one left a stamp still matching unchanged inputs
while the artifacts it vouched for were gone. Moving the work removes the
question, the stamp and the failure mode together.
Why it was safe to move, checked rather than assumed
The section is ~200 lines, and its coupling to the build script turned out to be
small enough to relocate rather than reconstruct:
AVOCADO_PREFIX,AVOCADO_SDK_PREFIX) comefrom the entrypoint prologue that every container run already sources. The
rest are deterministic paths, and the section now emits them itself, so it is
self-contained.
compression, losetup post-creation, device-tree overlays, LUKS room.
Nothing depended on build-time state that only the build script held.
provisionalready ran container commands and already read the var and stone config, so the
section slots in ahead of the existing hook call.
Shape
It moves to
commands/runtime/var_image.rsbehind one entry point taking anexplicit
VarImageContext. That context is the point, not just tidiness: it isprecisely the set of things a portable provisioning bundle has to carry, so a
later
avocado provision --bundle <path>becomes a matter of sourcing thecontext from a bundle instead of from a project.
Tests
The behaviour moved with the code rather than being dropped:
tight image — now asserted against the rendered section.
stone --overlay, and are inert when none aredeclared.
— the property a bundle depends on.
bash -non the rendered section, which is the check that catches a botchedheredoc.
buildkeeps a test that it emits none of the tail, anchored onstart-of-line commands because "stone bundle" also appears in a comment that
legitimately stays.
Addressed from Copilot review 1
All four findings were real; two of them were bugs that would have shipped.
$AVOCADO_PREFIX/output/runtimes/<rt>/var-stagingand anactivesymlink underlib/avocado/runtimes/.buildactually writes$AVOCADO_PREFIX/runtimes/<rt>and links
lib/avocado/active. It also readAVOCADO_NS_UUIDandAVOCADO_IMAGES_DIRfrom the environment, whichbuildused to export and nolonger does. The section now resolves the manifest the way the hash collection
does — the
activesymlink with afindfallback, becauseprovisiondoes notknow the
BUILD_IDthat names the directory — and exports what the python stepreads.
the_section_uses_the_paths_build_actually_writespins all of it,including that
$AVOCADO_PREFIX/outputappears nowhere.provisionablegate and flag. An earlier iteration of this workput a
--provisionableflag onbuildand gatedprovisionon a stamp. Thatdesign was dropped in favour of moving the work, but the flag, the
StampCommand::Provisionablevariant and the gate survived the rewrite. Sincebuildno longer writes that stamp, the gate would have refused everyprovision. All of it is gone.OUTPUT_DIR. Now derived from it.Correction: the OS bundle is OTA payload, not a provisioning artifact
An earlier revision of this PR moved the whole tail to
provision. That waswrong and was caught on hardware (QCS9075, UKI platform), not by any test here:
avocado-build-<target>hook is the kernel andinitramfs — it assembles the per-slot
uki_<slot>.efithat the stoneupdateblock names. With the hook only in
provision,buildemitted no bootartifacts at all.
os-bundle.aosis the OTA payload, andstone bundleis also what populatesos_artifacts. Without them the runtime manifest carried no OS payload.Observed:
avocado deployreported success, updated the extensions, switched theactive runtime, and left the board on its old kernel with
/varstillplaintext. Nothing reported that the OS had not been updated. That also made an
initramfs-only opt-in like
var.encryptunreachable over OTA, since its markerlives inside the initramfs.
The rule now applied: anything an OTA requires is at the tail of
runtime build; anything only provisioning consumes is at the start ofprovision.Ordering turned out to matter as much as placement. The manifest id is minted by
manifest_sectionduring build, so a bundle produced later by a separatecommand gets patched into a document whose id has not changed — a device already
holding that id skips the update, including the
os_bundlethat appearedinside it. Measured: a manifest genuinely carrying
os_bundlestill produced"Runtime already at target version, nothing to do." Building the bundle in the
same run that mints the id removes that failure mode.
This also fixes
sign_amf: command not found, which the moved code introduced:the OTA half calls
sign_amfand deliberately does not define it, inheriting thehelper from the build script. In
provisionthat call resolved to nothing andprovisioning died after the bundle was built. A test now asserts the helper is
defined before the call in the rendered script, and that the re-sign follows the
patch it exists for.
The one inference I could not test on hardware
stone bundleneeds a var partition size: every platform manifest declaresvaras
expand: "true"with no size, which is exactly what--partition-sizeoverrides, and stone fails hard without it — verified by running it:
partition 'var' omits size; no --partition-size override was supplied.The var image does not exist at build time, so the size is now declared from the
staged tree plus headroom rather than measured from the image. I checked that
nothing acts on it: no provisioning script reads the bundle (
avocado-provision-<target>has zero references to it, and the UFS flow injects raw images), an OTA never
repartitions, and the partition expands at provision time. If a
provision-from-bundle path is ever added, that number becomes real and must come
from the image — which is written at the call site.
Split
commands/runtime/var_image.rsnow exposes two halves over one context:render_ota_tail(spliced into the build script, inherits its variables) andrender_var_image(standalone, emits its own preamble). Tests assert the splitin both directions, because getting it backwards is not a build failure — it is
a
deploythat reports success and leaves the device on its old OS.